Do not treat match value patterns as isinstance checks
#20146
Conversation
This comment has been minimized.
This comment has been minimized.
|
I'm frankly not sure what part of the core team to ping here - reviewers of the original PR, @JukkaL? @randolf-scholz please also take a look if you have time, IIRC some recent PRs of yours were related to pattern matching? Primer finds nothing, apparently |
Well, many popular projects still support 3.9, but match-case requires at least 3.10, so that's not surprising. |
|
One thing that might be worth testing is if it interacts correctly with unions types and union patterns, e.g. from typing import Literal
def test1(x: Literal[1,2,3]) -> None:
match x:
case 1:
reveal_type(x)
case other:
reveal_type(x)
def test2(x: Literal[1,2,3]) -> None:
match x:
case 1:
reveal_type(x)
case 2:
reveal_type(x)
case 3:
reveal_type(x)
case other:
assert_never(x)
def test3(x: Literal[1,2,3]) -> None:
match x:
case 1 | 3:
reveal_type(x)
case other:
reveal_type(x)and possibly the same with enum arguments / enum patterns. |
|
I don't want to add any enum tests here, they won't be representative due to #19594 - I'd prefer to add |
This comment has been minimized.
This comment has been minimized.
| # like `if 1 < len(x) < 4: ...` | ||
| return reduce_conditional_maps(self.find_tuple_len_narrowing(node), use_meet=True) | ||
|
|
||
| def equality_type_narrowing_helper( |
There was a problem hiding this comment.
I see why you split this function, but it doesn't seem necessary?
There was a problem hiding this comment.
Hm? I split it because I want to reuse narrow_type_by_equality in another file, and there's no possibility of type(...) == ... checks as part of a checked pattern. Otherwise I'd have to create a fake node to call equality_type_narrowing_helper on it, and I'd rather avoid doing that
| def visit_value_pattern(self, o: ValuePattern) -> PatternType: | ||
| current_type = self.type_context[-1] | ||
| typ = self.chk.expr_checker.accept(o.expr) | ||
| typ = coerce_to_literal(typ) |
There was a problem hiding this comment.
Is this coerce_to_literal still necessary now that we use narrow_type_by_equality rather than conditional_types_with_intersection?
There was a problem hiding this comment.
It probably is; we are not in literal context, so coercion won't happen there for ==. Removing this will change the behavior significantly.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
3 participants
As discovered by @randolf-scholz in #20142,
mypytreats value patterns in match statement in a completely wrong way.From PEP 622:
Existing tests for the feature are invalid: for example,
must reveal
object, and testtestMatchValuePatternNarrowsasserts the opposite. Here's a runtime example:I have updated the existing tests accordingly.
The idea is that value patterns are essentially equivalent to
if foo == SomeValuechecks, notisinstancechecks modelled byconditional_types_wit_intersection.The original implementation was introduced in #10191.